-
-
Notifications
You must be signed in to change notification settings - Fork 131
feat: user deletion functionality #2212
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
BTW the lint error is unfixable! (take it as a challenge) It looks like the linter gets confused with the ternary operator inside the nested JSX structure. If there's no way to fix it, we can just disable the check for those lines. |
Looking for QA 👀 |
@m0wer this is going to take a minute. we are all kind of swamped and this is a big, potentially dangerous (although I'm sure it's fine) change. i'd guess you'll see a review when one of us needs a break from our own work. i personally might not be able to get to this for a couple weeks given some real life stuff. just wanted to give a heads up. this is something we want, so we'll get around to it eventually. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a quick review by looking at the code. I did not run or test the code.
prisma/migrations/20250610152839_deleted_user_earnings_job/migration.sql
Outdated
Show resolved
Hide resolved
@ekzyis ready for another round of QA! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Null Item Handling Error
A missing null check for item
causes a TypeError
when tx.item.findUnique
returns null (i.e., the item does not exist). This occurs when attempting to access item.userId
, preventing the intended "cannot zap deleted content" error.
api/paidAction/zap.js#L59-L63
stacker.news/api/paidAction/zap.js
Lines 59 to 63 in 89e1837
export async function perform ({ invoiceId, sats, id: itemId, ...args }, { me, cost, sybilFeePercent, tx }) { | |
const item = await tx.item.findUnique({ where: { id: parseInt(itemId) } }) | |
if (item.userId === USER_ID.delete) { | |
throw new Error('cannot zap deleted content') | |
} |
Was this report helpful? Give feedback by reacting with 👍 or 👎
Description
Implements account deletion functionality as requested in #603. This adds a comprehensive soft-delete system that allows users to permanently delete their accounts while preserving system integrity.
Closes #603
Key Features:
deletedAt
timestamp to maintain referential integrity@delete
user accountTechnical Implementation:
deletedAt
field to User model with database migrationdeleteAccount
GraphQL mutation with proper validationContent Handling:
@delete
user (USER_ID.delete = 106)deleteItemByAuthor
functionAuthentication & Session Management:
Balance & Rewards:
Screenshots
Additional Context
Key implementation decisions:
@delete
user and processed through existing deletion logic rather than being hashed or anonymizedThe authentication flow required updates in multiple places (NextAuth callbacks, SSR Apollo client, etc.) to properly handle deleted accounts across the application.
Checklist
Are your changes backwards compatible? Please answer below:
Yes, fully backwards compatible. Existing users are unaffected, and the
deletedAt
field is nullable. No breaking changes to existing APIs or database schema.On a scale of 1-10 how well and how have you QA'd this change and any features it might affect? Please answer below:
7/10 - Tested core deletion flow, authentication rejection, UI display, and balance handling. Would benefit from additional testing of edge cases around zap distribution to @delete user.
For frontend changes: Tested on mobile, light and dark mode? Please answer below:
Yes - The settings UI changes were tested on mobile and dark mode.
Did you introduce any new environment variables? If so, call them out explicitly here:
No new environment variables introduced. Uses existing
USER_ID.delete
constant for the special delete user account.